Skip to content
This repository has been archived by the owner on Nov 9, 2020. It is now read-only.

Returning proper exit code in cases where vmdkops admin command fails. #1543

Merged
merged 1 commit into from
Jul 6, 2017
Merged

Returning proper exit code in cases where vmdkops admin command fails. #1543

merged 1 commit into from
Jul 6, 2017

Conversation

pshahzeb
Copy link
Contributor

@pshahzeb pshahzeb commented Jul 6, 2017 •
edited
Loading

This PR is about returning proper exit code to indicate error when command fails to maintain consistency.
Removed redundant helper to print the error message and return it.

Fixes #1522

Testing:
test-e2e passes

2017/07/06 00:38:40 Destroying volume [vsanVol_volume_1016373@vsanDatastore]
2017/07/06 00:38:42 Removing policy [validPolicy] on esx [10.192.248.54]
OK: 28 passed
--- PASS: Test (1411.95s)
PASS
ok  	github.com/vmware/docker-volume-vsphere/tests/e2e	1411.957s
make: Leaving directory `/go/src/github.com/vmware/docker-volume-vsphere/vmdk_plugin'

Manual testing (Sample):

[root@sc2-rdops-vm07-dhcp-248-54:~] /usr/lib/vmware/vmdkops/bin/vmdkops_admin.py policy ls
Policy Name  Policy Content  Active
-----------  --------------  ------

[root@sc2-rdops-vm07-dhcp-248-54:~] /usr/lib/vmware/vmdkops/bin/vmdkops_admin.py policy rm --name IDONTEXIST
Error: IDONTEXIST does not exist
[root@sc2-rdops-vm07-dhcp-248-54:~] echo $?
2
[root@sc2-rdops-vm07-dhcp-248-54:~]

Returning proper exit code in cases where vmdkops admin command fails.
0b620a5 
1. Maintaining unambiguity through returning proper exit code when command fails along with message print.
2. Removed redundant helper to print the error message and return it.

Fixes vmware-archive#1522
@pshahzeb pshahzeb self-assigned this Jul 6, 2017
govint
govint approved these changes Jul 6, 2017
View reviewed changes
Copy link
Contributor

@govint govint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

shaominchen
shaominchen reviewed Jul 6, 2017
View reviewed changes
esx_service/cli/vmdkops_admin.py
@@ -958,7 +954,7 @@ def tenant_create(args):
privileges=[])

if error_info:
return operation_fail(error_info.msg)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is operation_fail still being used? If not, we should remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I have removed it. See line 902

shaominchen
shaominchen approved these changes Jul 6, 2017
View reviewed changes
Copy link
Contributor

@shaominchen shaominchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@pshahzeb pshahzeb merged commit 6f4d7d7 into vmware-archive:master Jul 6, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@shaominchen shaominchen shaominchen approved these changes

@govint govint govint approved these changes

Assignees

@pshahzeb pshahzeb

Labels
cla-not-required
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pshahzeb @govint @shaominchen @vmwclabot